-
Notifications
You must be signed in to change notification settings - Fork 6
fix: Resolve nested relations #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #66 +/- ##
============================================
+ Coverage 93.79% 93.80% +0.01%
- Complexity 470 471 +1
============================================
Files 73 73
Lines 1643 1646 +3
============================================
+ Hits 1541 1544 +3
Misses 102 102 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/Resolver/StoryResolver.php
Outdated
| foreach ($target as &$value) { | ||
| if (\is_string($value) && \array_key_exists($value, $relationMap)) { | ||
| $value = $relationMap[$value]; | ||
| return $this->doResolve($target, $relationMap, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are two things done:
- deep resolving of relations
- not to replace the uuids themselves
This would fix my both opened issues. But I think you could simplify this approach a bit.
We have implemented our own StoryResolver in the meantime (I had not enough time yet to create a PR with our version) and we did it like this:
// resolve relations which may be included in the resolved relations themselves
$this->doResolve($relationMap, $relationMap);
// resolve relations in the main target
$this->doResolve($target, $relationMap);
return $target;
private function doResolve(array &$target, array $relationMap): void
//...
// uuid check here like it is done in this PR
We are passing the $target as reference because our API responses are quite huge and we want to lower the memory usage on our servers.
In this case you do not need the $seen parameter and can assume that the resolved relations are already resolved when the main stories are looped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there sorry for the delay i found a more memory saving appoach. if you want to test it feel free.
6729710 to
e77eee9
Compare
be0778e to
3f09650
Compare
closes #63 #62